feat: add implementation of stats/base/dists/halfnormal/kurtosis#9771
feat: add implementation of stats/base/dists/halfnormal/kurtosis#9771Planeshifter merged 4 commits intostdlib-js:developfrom
stats/base/dists/halfnormal/kurtosis#9771Conversation
---
type: pre_commit_static_analysis_report
description: Results of running static analysis checks when committing changes.
report:
- task: lint_filenames
status: passed
- task: lint_editorconfig
status: passed
- task: lint_markdown
status: passed
- task: lint_package_json
status: passed
- task: lint_repl_help
status: passed
- task: lint_javascript_src
status: passed
- task: lint_javascript_cli
status: na
- task: lint_javascript_examples
status: passed
- task: lint_javascript_tests
status: passed
- task: lint_javascript_benchmarks
status: passed
- task: lint_python
status: na
- task: lint_r
status: na
- task: lint_c_src
status: missing_dependencies
- task: lint_c_examples
status: missing_dependencies
- task: lint_c_benchmarks
status: missing_dependencies
- task: lint_c_tests_fixtures
status: na
- task: lint_shell
status: na
- task: lint_typescript_declarations
status: passed
- task: lint_typescript_tests
status: passed
- task: lint_license_headers
status: passed
---
Planeshifter
left a comment
There was a problem hiding this comment.
Thanks for opening this PR!
| // VARIABLES // | ||
|
|
||
| var PI = require( '@stdlib/constants/float64/pi' ); | ||
| var isnan = require( '@stdlib/math/base/assert/is-nan' ); |
There was a problem hiding this comment.
This section header should be // MODULES // since it contains require statements. The // VARIABLES // section is for computed constants and other variable declarations.
| // VARIABLES // | |
| var PI = require( '@stdlib/constants/float64/pi' ); | |
| var isnan = require( '@stdlib/math/base/assert/is-nan' ); | |
| // MODULES // | |
| var PI = require( '@stdlib/constants/float64/pi' ); | |
| var isnan = require( '@stdlib/math/base/assert/is-nan' ); |
| ) { | ||
| return 0.0 / 0.0; // NaN | ||
| } | ||
| return ( 8.0 * ( STDLIB_CONSTANT_FLOAT64_PI - 3.0 ) ) / ( ( STDLIB_CONSTANT_FLOAT64_PI - 2.0 ) *( STDLIB_CONSTANT_FLOAT64_PI - 2.0 ) ); |
There was a problem hiding this comment.
Missing space before the multiplication operator. Let's match the stdlib C style guide.
| return ( 8.0 * ( STDLIB_CONSTANT_FLOAT64_PI - 3.0 ) ) / ( ( STDLIB_CONSTANT_FLOAT64_PI - 2.0 ) *( STDLIB_CONSTANT_FLOAT64_PI - 2.0 ) ); | |
| return ( 8.0 * ( STDLIB_CONSTANT_FLOAT64_PI - 3.0 ) ) / ( ( STDLIB_CONSTANT_FLOAT64_PI - 2.0 ) * ( STDLIB_CONSTANT_FLOAT64_PI - 2.0 ) ); |
| b.tic(); | ||
| for ( i = 0; i < b.iterations; i++ ) { | ||
| // Generate valid scale parameters: | ||
| sigma = ( randu() * 10.0 ) + 0.1; | ||
| y = kurtosis( sigma ); |
There was a problem hiding this comment.
The random data generation should happen outside the timing loop. Per latest stdlib benchmark conventions, pre-generate values before b.tic() and cycle through them using modulo indexing.
| "directories": { | ||
| "benchmark": "./benchmark", | ||
| "doc": "./docs", | ||
| "example": "./examples", | ||
| "lib": "./lib", | ||
| "test": "./test" | ||
| }, |
There was a problem hiding this comment.
Since this package has a C implementation with a .gyp file, we need to add "gypfile": true and include the include and src directories. See @stdlib/stats/base/dists/levy/pdf/package.json for reference.
The directories section should look like:
"directories": {
"benchmark": "./benchmark",
"doc": "./docs",
"example": "./examples",
"include": "./include",
"lib": "./lib",
"src": "./src",
"test": "./test"
}And add "gypfile": true after line 16.
| sigma = data.sigma; | ||
| for ( i = 0; i < sigma.length; i++ ) { | ||
| y = kurtosis( sigma[i] ); | ||
| if ( expected[i] !== null) { |
There was a problem hiding this comment.
Missing space before the closing parenthesis to match stdlib style.
| if ( expected[i] !== null) { | |
| if ( expected[i] !== null ) { |
| for ( i = 0; i < 10; i++ ) { | ||
| sigma = randu() * 20.0; | ||
| y = kurtosis( sigma ); | ||
| console.log( 'σ: %lf, Kurt(σ): %lf', sigma, y ); |
There was a problem hiding this comment.
JavaScript's console.log doesn't interpret %lf as a format specifier - it just prints it literally. Use %d for numbers instead.
| console.log( 'σ: %lf, Kurt(σ): %lf', sigma, y ); | |
| console.log( 'σ: %d, Kurt(σ): %d', sigma, y ); |
| for ( i = 0; i < 10; i++ ) { | ||
| sigma = randu() * 20.0; | ||
| y = kurtosis( sigma ); | ||
| console.log( 'σ: %lf, Kurt(σ): %lf', sigma, y ); |
There was a problem hiding this comment.
Same issue here - %lf won't work as a format specifier in JavaScript. Use %d instead.
| console.log( 'σ: %lf, Kurt(σ): %lf', sigma, y ); | |
| console.log( 'σ: %d, Kurt(σ): %d', sigma, y ); |
| } | ||
| b.pass( 'benchmark finished' ); | ||
| b.end(); | ||
| }); |
There was a problem hiding this comment.
This package is missing benchmark/benchmark.native.js which is needed for benchmarking the native addon. See @stdlib/stats/base/dists/levy/pdf/benchmark/benchmark.native.js for a reference implementation.
Coverage Report
The above coverage report was generated for the changes in this PR. |
---
type: pre_commit_static_analysis_report
description: Results of running static analysis checks when committing changes.
report:
- task: lint_filenames
status: passed
- task: lint_editorconfig
status: passed
- task: lint_markdown
status: passed
- task: lint_package_json
status: passed
- task: lint_repl_help
status: na
- task: lint_javascript_src
status: passed
- task: lint_javascript_cli
status: na
- task: lint_javascript_examples
status: passed
- task: lint_javascript_tests
status: passed
- task: lint_javascript_benchmarks
status: passed
- task: lint_python
status: na
- task: lint_r
status: na
- task: lint_c_src
status: missing_dependencies
- task: lint_c_examples
status: na
- task: lint_c_benchmarks
status: na
- task: lint_c_tests_fixtures
status: na
- task: lint_shell
status: na
- task: lint_typescript_declarations
status: passed
- task: lint_typescript_tests
status: na
- task: lint_license_headers
status: passed
---
|
@Planeshifter I’ve addressed the requested changes. I’d appreciate it if you could take another look when convenient thanks again for your help. |
---
type: pre_commit_static_analysis_report
description: Results of running static analysis checks when committing changes.
report:
- task: lint_filenames
status: passed
- task: lint_editorconfig
status: passed
- task: lint_markdown
status: na
- task: lint_package_json
status: na
- task: lint_repl_help
status: na
- task: lint_javascript_src
status: na
- task: lint_javascript_cli
status: na
- task: lint_javascript_examples
status: na
- task: lint_javascript_tests
status: passed
- task: lint_javascript_benchmarks
status: na
- task: lint_python
status: missing_dependencies
- task: lint_r
status: na
- task: lint_c_src
status: na
- task: lint_c_examples
status: na
- task: lint_c_benchmarks
status: na
- task: lint_c_tests_fixtures
status: na
- task: lint_shell
status: na
- task: lint_typescript_declarations
status: passed
- task: lint_typescript_tests
status: na
- task: lint_license_headers
status: passed
---
---
type: pre_commit_static_analysis_report
description: Results of running static analysis checks when committing changes.
report:
- task: lint_filenames
status: passed
- task: lint_editorconfig
status: passed
- task: lint_markdown
status: passed
- task: lint_package_json
status: na
- task: lint_repl_help
status: na
- task: lint_javascript_src
status: na
- task: lint_javascript_cli
status: na
- task: lint_javascript_examples
status: passed
- task: lint_javascript_tests
status: passed
- task: lint_javascript_benchmarks
status: na
- task: lint_python
status: missing_dependencies
- task: lint_r
status: na
- task: lint_c_src
status: na
- task: lint_c_examples
status: na
- task: lint_c_benchmarks
status: na
- task: lint_c_tests_fixtures
status: na
- task: lint_shell
status: na
- task: lint_typescript_declarations
status: passed
- task: lint_typescript_tests
status: na
- task: lint_license_headers
status: passed
---
Planeshifter
left a comment
There was a problem hiding this comment.
Let's get this one merged. Thanks!
PR Commit MessagePlease review the above commit message and make any necessary adjustments. |
PR-URL: stdlib-js#9771 Ref: stdlib-js#9416 Co-authored-by: Philipp Burckhardt <pburckhardt@outlook.com> Reviewed-by: Philipp Burckhardt <pburckhardt@outlook.com>
Progresses #9416.
Description
This pull request:
Related Issues
This pull request progresses the following related issue:
@stdlib/stats/base/dists/halfnormalpackage #9416Questions
No.
Other
No.
Checklist
AI Assistance
If you answered "yes" above, how did you use AI assistance?
Disclosure
I used llm , wikipedia and other mathematical websites in a limited capacity to verify the mathematical formula for half-normal excess kurtosis and to cross-check references. All code, tests, and documentation were authored manually.
@stdlib-js/reviewers